Improve source heading buttons and interactions. #3202

hgluka · 2023-10-04T11:08:49Z

Description

Change next and previous source glyphs, mute the color when there is no next or previous source and show keybinding on hover.

Depends on atlas-engineer/prompter#24.

Fixes #2852.
Fixes #2556.

Discussion

It might be useful in general to have an inactive style class predefined.
cc @aartaka @lansingthomas

Checklist:

Everything in this checklist is required for each PR. Please do not approve a PR that does not have all of these items.

Git hygiene:
- I have pulled from master before submitting this PR
- There are no merge conflicts.

I've added the new dependencies as:

ASDF dependencies,

Git submodules,

cd /path/to/nyxt/checkout
git submodule add https://gitlab.common-lisp.net/nyxt/py-configparser _build/py-configparser

and Guix dependencies.

My code follows the style guidelines for Common Lisp code. See:
- Norvig & Pitman's Tutorial on Good Lisp Programming Style (PDF)
- Google Common Lisp Style Guide
I have performed a self-review of my own code.
My code has been reviewed by at least one peer. (The peer review to approve a PR counts. The reviewer must download and test the code.)
Documentation:
- All my code has docstrings and :documentations written in the aforementioned style. (It's OK to skip the docstring for really trivial parts.)
- I have updated the existing documentation to match my changes.
- I have commented my code in hard-to-understand areas.
- I have updated the changelog.lisp with my changes if it's anything user-facing (new features, important bug fix, compatibility breakage).
  - Changelog update should be a separate commit.
- I have added a migration.lisp entry for all compatibility-breaking changes.
- (If this changes something about the features showcased on Nyxt website) I have these changes described in the new/existing article at Nyxt website or will notify one of maintainters to do so.
Compilation and tests:
- My changes generate no new warnings.
- I have added tests that prove my fix is effective or that my feature works. (If possible.)
- I ran the tests locally ((asdf:test-system :nyxt) and (asdf:test-system :nyxt/gi-gtk)) and they pass.

lansingthomas · 2023-10-04T17:06:28Z

Let's just keep the solution local for today :D

no need for a special class in other places yet.

thx Luka

jmercouris · 2023-10-04T19:43:05Z

Will review ASAP!

source/prompt-buffer.lisp

hgluka · 2023-10-06T09:39:17Z

I made a PR in the prompter library to add next-source-p and previous-source-p, so this PR now depends on atlas-engineer/prompter#24.

aadcg · 2023-10-10T20:15:11Z

@hgluka I've bumped the submodule.

To conclude this PR:

Read carefully button to trigger the toggle-attributes-display menu #2852 and update interaction states for the prev/next source arrows in prompt area #3139;
Validate the UI with @lansingthomas by sharing screenshots and/or short videos;
Changelog.

I'll merge it when the patch to Guix has been accepted.

hgluka · 2023-10-17T11:03:02Z

Here's a screenshot, by the way:

@lansingthomas Let me know what you think. When you approve, I'll add a changelog entry and it will be ready to merge.

lansingthomas · 2023-10-17T15:37:19Z

Can you send me a video? -- alternatively we can screen share tomorrow (or today if you are willing).

hgluka · 2023-10-17T16:53:40Z

I'll share my screen with you tomorrow. That way, we can work out any issues in real time.

On another note, I'm not sure why these builds are failing. There are a lot of Connection timed out messages when fetching the submodules.
@aadcg Any ideas?

aadcg · 2023-10-17T18:36:02Z

@hgluka no idea. But I don't think we should care either, because CI is mostly useless. All it does can be trivially replicated locally. My intuition tells me that it will start magically to work again for no apparent reason. If it doesn't, we'll get to that.

aadcg · 2023-10-18T19:41:44Z

The patch has been accepted on Guix meaning that we can move forward.

What's the status here? We should have this one ready by the end of the week so that it lands on the next release. Thanks.

lansingthomas · 2023-10-18T21:20:01Z

Sorry I am the bottleneck for this at the moment. -- we are scheduled for tomorrow for another interaction review.

my concerns are about the size of the icons, functionally it is looking good.

lansingthomas · 2023-10-19T17:26:47Z

had a nice convo with Luka -- we agreed on styling for the arrows. he is reporting an observed bug with the +/-

I approve of these changes.

Thx @hgluka

hgluka · 2023-10-19T18:11:07Z

Yep, this is ready to review/merge @aadcg.

aadcg · 2023-10-20T08:28:45Z

I will review when #3231 is fixed in this PR. Please let me know.

#3231 could have been spotted without @lansingthomas's intervention. That's part of the self-reviewing process expected from PRs. The work the team outputs can be as good as its ability to self-criticism.

hgluka · 2023-10-30T11:18:44Z

@aadcg This PR is ready for a final review/merge.

aadcg · 2023-10-31T21:19:47Z

There's a UI glitch in this PR. Sometimes, both arrows are inactive. I need to investigate.

aadcg · 2023-11-08T10:05:54Z

There's a UI glitch in this PR. Sometimes, both arrows are inactive. I need to investigate.

Unfortunately, there's no easy way around it. Commit 28179e7 provides a detailed explanation. Therefore, this PR no longer closes #3139.

@jmercouris please review.

aadcg · 2023-11-13T20:32:08Z

@jmercouris friendly ping.

jmercouris · 2023-11-13T21:31:04Z

yes, sorry for the delay!

Change button glyphs for `next-source`, `previous-source` and `toggle-attributes-display`. Show keybinding on hover. Not all added glyphs are being used, but they may be useful in the future. Move `toggle-attributes-source` button to the right. Mute next/previous source arrow when next/previous source is unavailable. Review CSS.

The previous commit introduces a UI glitch where the left arrow down button is drawn as inactive by a brief moment. The root cause isn't related to the implementation of the feature itself, but due to the poor architecture design of update-prompt-input. Steps to reproduce: - Invoke execute-command - Run `(update-prompt-input (current-prompt-buffer) "fo")` The issue can be understood by inspecting the calls to `(prompt-render-suggestions prompt-buffer)` in `update-prompt-input`.

aadcg requested a review from jmercouris October 4, 2023 19:59

aadcg reviewed Oct 4, 2023

View reviewed changes

source/prompt-buffer.lisp Outdated Show resolved Hide resolved

hgluka force-pushed the prompt-buffer-arrows branch from 3d0e117 to 525f7ce Compare October 6, 2023 09:36

hgluka mentioned this pull request Oct 10, 2023

Add next-source-p and previous-source-p. atlas-engineer/prompter#24

Merged

hgluka force-pushed the prompt-buffer-arrows branch from 525f7ce to 99a1719 Compare October 10, 2023 15:42

aadcg mentioned this pull request Oct 10, 2023

button to trigger the toggle-attributes-display menu #2852

Closed

5 tasks

aadcg force-pushed the prompt-buffer-arrows branch from 99a1719 to bb6cde5 Compare October 10, 2023 20:11

hgluka changed the title ~~Improve next and previous source buttons in prompt buffer.~~ Improve source heading buttons and interactions. Oct 11, 2023

hgluka force-pushed the prompt-buffer-arrows branch from 0459e44 to e01203a Compare October 19, 2023 18:08

aadcg mentioned this pull request Oct 20, 2023

toggle-attributes-display always uses current-source #3231

Closed

hgluka force-pushed the prompt-buffer-arrows branch from e01203a to 44d6074 Compare October 25, 2023 13:25

aadcg force-pushed the prompt-buffer-arrows branch from 44d6074 to c00f7f3 Compare October 31, 2023 21:18

aadcg force-pushed the prompt-buffer-arrows branch from c00f7f3 to 2bef781 Compare November 8, 2023 10:02

aadcg force-pushed the prompt-buffer-arrows branch from 2bef781 to 7aac381 Compare November 8, 2023 10:30

aadcg and others added 5 commits November 26, 2023 21:28

gitmodules: Update prompter.

399d481

prompt-buffer: Review style.

c43cb59

changelog: Mention source heading UI improvement.

6450b4a

aadcg force-pushed the prompt-buffer-arrows branch from 7aac381 to 6450b4a Compare November 26, 2023 19:33

aadcg merged commit bed40cd into master Nov 26, 2023
2 checks passed

aadcg deleted the prompt-buffer-arrows branch November 26, 2023 19:34

aadcg referenced this pull request Jan 11, 2024

prompt-buffer.lisp: Correct prompt buffer arrows.

37537c7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve source heading buttons and interactions. #3202

Improve source heading buttons and interactions. #3202

hgluka commented Oct 4, 2023 •

edited by aadcg

Loading

lansingthomas commented Oct 4, 2023

jmercouris commented Oct 4, 2023

hgluka commented Oct 6, 2023

aadcg commented Oct 10, 2023

hgluka commented Oct 17, 2023

lansingthomas commented Oct 17, 2023

hgluka commented Oct 17, 2023

aadcg commented Oct 17, 2023

aadcg commented Oct 18, 2023 •

edited

Loading

lansingthomas commented Oct 18, 2023

lansingthomas commented Oct 19, 2023

hgluka commented Oct 19, 2023

aadcg commented Oct 20, 2023 •

edited

Loading

hgluka commented Oct 30, 2023

aadcg commented Oct 31, 2023

aadcg commented Nov 8, 2023 •

edited

Loading

aadcg commented Nov 13, 2023

jmercouris commented Nov 13, 2023

Improve source heading buttons and interactions. #3202

Improve source heading buttons and interactions. #3202

Conversation

hgluka commented Oct 4, 2023 • edited by aadcg Loading

Description

Discussion

Checklist:

lansingthomas commented Oct 4, 2023

jmercouris commented Oct 4, 2023

hgluka commented Oct 6, 2023

aadcg commented Oct 10, 2023

hgluka commented Oct 17, 2023

lansingthomas commented Oct 17, 2023

hgluka commented Oct 17, 2023

aadcg commented Oct 17, 2023

aadcg commented Oct 18, 2023 • edited Loading

lansingthomas commented Oct 18, 2023

lansingthomas commented Oct 19, 2023

hgluka commented Oct 19, 2023

aadcg commented Oct 20, 2023 • edited Loading

hgluka commented Oct 30, 2023

aadcg commented Oct 31, 2023

aadcg commented Nov 8, 2023 • edited Loading

aadcg commented Nov 13, 2023

jmercouris commented Nov 13, 2023

hgluka commented Oct 4, 2023 •

edited by aadcg

Loading

aadcg commented Oct 18, 2023 •

edited

Loading

aadcg commented Oct 20, 2023 •

edited

Loading

aadcg commented Nov 8, 2023 •

edited

Loading